Fix checks for hardware intrinsics special casing in IL interpreter #38835
Conversation
yuchong-pan
added
area-CodeGen-coreclr
yuchong-pan
requested review from
noahfalk,
BruceForstall,
tommcdon,
josalem and
JulieLeeMSFT
BruceForstall
removed
the
area-CodeGen-coreclr
| #endif // defined(TARGET_X86) || defined(TARGET_AMD64) | ||
| #endif // _DEBUG | ||
| strcmp(methToCall->GetName(), "get_IsSupported") == 0) | ||
| strcmp(methodName, "get_IsSupported") == 0 |
There was a problem hiding this comment.
Is this going to get nested classes such as: System.Runtime.Intrinsics.Arm.AdvSimd.Arm64.get_IsSupported?
There was a problem hiding this comment.
Likewise, given that DoGetIsSupported() just returns false, is there any reason why we aren't just special casing all of these get_IsSupported functions?
There was a problem hiding this comment.
Is this going to get nested classes such as: System.Runtime.Intrinsics.Arm.AdvSimd.Arm64.get_IsSupported?
I think these might need to be strncmps to handle all those cases correctly and just check the first 29 chars.
is there any reason why we aren't just special casing all of these get_IsSupported functions?
Could you expand what you mean by "special casing" here? I think this is intended to be a temporary solution until a more production quality approach can be designed. See this comment for context on what kinds of cases need to be covered by a more robust solution.
There was a problem hiding this comment.
I was just indicating that they were being handled here to explicitly return false.
There was a problem hiding this comment.
Gotcha! Yeah, the intention was to special case all the HW intrinsics' get_IsSupported methods to return false until the interpreter has a more robust solution.
ghost
locked as resolved and limited conversation to collaborators
In #38673, @BruceForstall indicated that
EEClass::GetDebugClassName()in the checks for theget_IsSupported()methods of hardware intrinsics is only available in_DEBUGbuilds. Hence I changed the checks to obtain the method name, class name, and namespace name from the generally availableCEEInfo::getMethodNameeFromMetadatamethod.